Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Classes #2813

Merged
merged 68 commits into from
Aug 30, 2023
Merged

Classes #2813

merged 68 commits into from
Aug 30, 2023

Conversation

vixalien
Copy link
Contributor

What:

Use class throughout the project instead of using .prototype.__proto__

Why:

As talked about in #2745, Classes are better supported than __proto__. And this brings stylus on track to have a good and up-to-date codebase.

Checklist:

  • Documentation N/A
  • Unit Tests N/a
  • Code complete
  • Changelog

@netlify
Copy link

netlify bot commented Aug 14, 2023

Deploy Preview for stylus-docs canceled.

Name Link
🔨 Latest commit 9610e4e
🔍 Latest deploy log https://app.netlify.com/sites/stylus-docs/deploys/64da21cfd709590008a50563

@vixalien vixalien mentioned this pull request Aug 14, 2023
4 tasks
@iChenLei
Copy link
Member

Can you add deno test for this pr? Thanks ! For Stylus, this is a significant change, so it also implies that it's unlikely to complete the review and merge in a short period of time. Perhaps we should also consider releasing an alpha version to test community feedback. @vixalien

@vixalien
Copy link
Contributor Author

@iChenLei I added deno tests.

And yes, I can see this is a huge PR and will need time to review. you can release an alpha if necessary.

also note that the build still fails because of @adobe/css-tools

@iChenLei
Copy link
Member

@iChenLei I added deno tests.

And yes, I can see this is a huge PR and will need time to review. you can release an alpha if necessary.

also note that the build still fails because of @adobe/css-tools

Could you help with downgrading @adobe/css-tools to support Node 10+? Using a fixed version might be a good approach, but I'm also considering whether to raise Stylus support to Node 14+. And again, big thanks to you.

@vixalien
Copy link
Contributor Author

vixalien commented Aug 14, 2023

Could you help with downgrading @adobe/css-tools to support Node 10+

Downgraded @adobe/css-tools to ~4.2.0 (the latest is 4.3.0)

but I'm also considering whether to raise Stylus support to Node 14+

If you do so, I could work on bringing ES Modules support to stylus if that's desirable

And again, big thanks to you.

no worries! thank you too for taking care of this PR and giving feedback

@iChenLei
Copy link
Member

excellent work ! @vixalien

@iChenLei iChenLei mentioned this pull request Aug 19, 2023
21 tasks
lib/nodes/null.js Show resolved Hide resolved
lib/nodes/null.js Show resolved Hide resolved
@iChenLei iChenLei merged commit 50b0a33 into stylus:dev Aug 30, 2023
18 checks passed
@vixalien vixalien deleted the classes branch August 30, 2023 07:17
@vixalien vixalien mentioned this pull request Nov 8, 2023
4 tasks
iChenLei added a commit that referenced this pull request Nov 18, 2023
* fix super call in object.js

the super.operate.call syntax doesn't pass in the `this` argument.

The code before the classes #2813 PR was:

```js
Node.prototype.operate.call(this, op, right)
```

and the code was incorrectly transformed into

```js
super.operate.call(op, right);
```

* directly call super methods

instead of doing `super.method.call(this, ...args)`

* chore: add test cases

* chore: add eos

---------

Co-authored-by: Angelo Verlain <geoangercola@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants